Skip to content

Add versification warnings for invalid chapter or verse numbers in USFM#381

Open
pmachapman wants to merge 1 commit intomasterfrom
invalid_verse_or_chapter
Open

Add versification warnings for invalid chapter or verse numbers in USFM#381
pmachapman wants to merge 1 commit intomasterfrom
invalid_verse_or_chapter

Conversation

@pmachapman
Copy link
Collaborator

@pmachapman pmachapman commented Feb 4, 2026 •
edited by ddaspit
Loading

Fixes: #372

Example USFM:

\id MAN
\c 1.
\v v1 Verse 1

Example warnings:

warnings: [
  "USFM versification error in project TEA, expected verse “”, actual verse “MAN 1.”, mismatch type InvalidChapterNumber (parallel corpus 6983d7b5415e5f0ee3585264, monolingual corpus 6983d7b5415e5f0ee3585260)",
  "USFM versification error in project TEA, expected verse “”, actual verse “MAN -1:v1”, mismatch type InvalidVerseNumber (parallel corpus 6983d7b5415e5f0ee3585264, monolingual corpus 6983d7b5415e5f0ee3585260)",
]

This change is Reviewable

@codecov-commenter
Copy link

codecov-commenter commented Feb 4, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.79%. Comparing base (611fbf2) to head (5ca0162).

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #381      +/-   ##
==========================================
+ Coverage   72.73%   72.79%   +0.06%     
==========================================
  Files         423      423              
  Lines       36029    36089      +60     
  Branches     4969     4974       +5     
==========================================
+ Hits        26205    26271      +66     
+ Misses       8733     8727       -6     
  Partials     1091     1091

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pmachapman pmachapman force-pushed the invalid_verse_or_chapter branch from 39cf6ef to 5ca0162 Compare February 5, 2026 00:51
ddaspit
ddaspit reviewed Feb 6, 2026
View reviewed changes
Copy link
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warning messages are a bit awkward. How difficult would it be to generate messages that are more appropriate for the new warnings?

@ddaspit reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Enkidu93).

@pmachapman pmachapman mentioned this pull request Feb 9, 2026
Open
pmachapman
pmachapman commented Feb 9, 2026
View reviewed changes
Copy link
Collaborator Author

@pmachapman pmachapman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warning messages are a bit awkward. How difficult would it be to generate messages that are more appropriate for the new warnings?

Good idea. That requires a modification to Serval - see sillsdev/serval/#871

@pmachapman made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Enkidu93).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ddaspit ddaspit ddaspit left review comments

@Enkidu93 Enkidu93 Awaiting requested review from Enkidu93

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

The USFM parser should trap the Trace warnings from VerseRef

3 participants

@pmachapman @codecov-commenter @ddaspit